Add default_colors API to Theme DSL for setting terminal fg/bg - #235
Conversation
Themes can now declare default foreground/background colors via t.default_colors(foreground:, background:) which calls Curses.assume_default_colors during activation. Themes without default_colors reset to terminal defaults (-1, -1). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Both original themes set Normal fg/bg; our ports were missing it. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Emacs-compatible interactive commands for changing the terminal's default foreground or background color independently. Window now tracks current default colors so each command can update one without resetting the other. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds support for themes to declare terminal default foreground/background colors (via Theme DSL) and applies them during theme activation to influence how “default” (-1) colors are rendered.
Changes:
- Introduces
Theme#default_colors(foreground:, background:)and applies it onTheme#activate(resetting to terminal defaults when unspecified). - Updates several built-in themes to specify default fg/bg.
- Adds commands for interactively setting default foreground/background colors, plus tests for theme default-colors behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
lib/textbringer/theme.rb |
Adds Theme DSL + activation-time application of default fg/bg colors. |
lib/textbringer/window.rb |
Tracks cached default fg/bg to support partial updates and applies them via Curses.assume_default_colors. |
lib/textbringer/commands/windows.rb |
Adds set_foreground_color / set_background_color commands. |
lib/textbringer/themes/tokyonight.rb |
Declares theme default fg/bg. |
lib/textbringer/themes/sonokai.rb |
Declares theme default fg/bg. |
lib/textbringer/themes/molokai.rb |
Declares theme default fg/bg. |
lib/textbringer/themes/gruvbox.rb |
Declares theme default fg/bg. |
lib/textbringer/themes/github.rb |
Declares theme default fg/bg. |
lib/textbringer/themes/catppuccin.rb |
Declares theme default fg/bg. |
test/textbringer/test_theme.rb |
Adds tests covering activation setting/resetting default colors and erroring on unknown palette entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fg = "default" | ||
| bg = "default" | ||
| end | ||
| Curses.assume_default_colors(Color[fg], Color[bg]) |
There was a problem hiding this comment.
Theme#activate applies default colors via Curses.assume_default_colors directly, but Window.set_default_colors now keeps its own cached @@default_fg/@@default_bg (used by the new set_foreground_color/set_background_color commands). After activating a theme that sets default_colors, Window’s cache remains stale (still "default"), so setting only fg/bg later will unexpectedly reset the other side back to terminal default. Consider routing through Window.set_default_colors (or otherwise synchronizing Window’s cached defaults) when applying theme default colors, including the reset-to-default case.
| Curses.assume_default_colors(Color[fg], Color[bg]) | |
| Window.set_default_colors(fg, bg) |
| @@default_fg = fg if fg | ||
| @@default_bg = bg if bg | ||
| Curses.assume_default_colors(Color[@@default_fg], Color[@@default_bg]) |
There was a problem hiding this comment.
Window.set_default_colors updates @@default_fg/@@default_bg before validating the inputs. If Color[...] raises (e.g., user hits Enter and read_from_minibuffer returns ""), the invalid value is still cached, and subsequent calls that pass nil for that side will keep failing unexpectedly. Suggest validating/converting fg/bg first (and/or treating blank strings as nil) and only updating the cached defaults after Curses.assume_default_colors succeeds.
| @@default_fg = fg if fg | |
| @@default_bg = bg if bg | |
| Curses.assume_default_colors(Color[@@default_fg], Color[@@default_bg]) | |
| # Treat blank strings (e.g., when the user just hits Enter) as nil so | |
| # they do not corrupt the cached default colors. | |
| fg = nil if fg.respond_to?(:empty?) && fg.empty? | |
| bg = nil if bg.respond_to?(:empty?) && bg.empty? | |
| # Determine the effective colors for this call without mutating | |
| # the cached defaults yet. | |
| new_fg = fg || @@default_fg | |
| new_bg = bg || @@default_bg | |
| # Validate/convert the colors first; if this raises, the cached | |
| # defaults remain unchanged. | |
| fg_color = Color[new_fg] | |
| bg_color = Color[new_bg] | |
| Curses.assume_default_colors(fg_color, bg_color) | |
| # Only update cached defaults after successful application. | |
| @@default_fg = new_fg if fg | |
| @@default_bg = new_bg if bg |
| define_command(:set_foreground_color, doc: <<~EOD) do | ||
| Set the default foreground color. | ||
| EOD | ||
| |color = read_from_minibuffer("Foreground color: ")| | ||
| Window.set_default_colors(color, nil) | ||
| end | ||
|
|
||
| define_command(:set_background_color, doc: <<~EOD) do | ||
| Set the default background color. | ||
| EOD | ||
| |color = read_from_minibuffer("Background color: ")| | ||
| Window.set_default_colors(nil, color) | ||
| end |
There was a problem hiding this comment.
These commands call read_from_minibuffer without a default value; when the user submits an empty input, read_from_minibuffer returns "" (see lib/textbringer/utils.rb:169-174), which will currently cause Window.set_default_colors to raise and (with the new caching) potentially leave an invalid cached default. Consider providing a default (e.g., "default") or mapping empty input to nil/"default" before calling Window.set_default_colors.
| @@default_fg = fg if fg | ||
| @@default_bg = bg if bg | ||
| Curses.assume_default_colors(Color[@@default_fg], Color[@@default_bg]) |
There was a problem hiding this comment.
The updated Window.set_default_colors now supports partial updates via nil (used by set_foreground_color/set_background_color), but test/textbringer/test_window.rb only covers the two-argument case. Adding tests for (a) preserving the previous bg/fg when passing nil and (b) not poisoning the cached defaults on invalid/blank input would help prevent regressions.
| @@default_fg = fg if fg | |
| @@default_bg = bg if bg | |
| Curses.assume_default_colors(Color[@@default_fg], Color[@@default_bg]) | |
| new_fg = fg || @@default_fg | |
| new_bg = bg || @@default_bg | |
| Curses.assume_default_colors(Color[new_fg], Color[new_bg]) | |
| @@default_fg = new_fg | |
| @@default_bg = new_bg |
Curses.assume_default_colors only affects color_pair 0 and does not reliably update the display when called after initial setup (e.g. to keep terminal background transparency while using theme fg). Now set_default_colors also defines a :default face with the resolved fg/bg colors. All fallback sites in Window that previously used attr_set(0, 0) now use Face[:default], ensuring theme colors are applied consistently even when one component is overridden. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Resolve colors via Color[] before updating @@default_fg/@@default_bg so invalid input does not corrupt the cache - Treat empty strings as nil (no-op) to handle blank minibuffer input - Add tests for partial update (nil preserves other side) and empty string handling Co-Authored-By: Claude Opus 4.6 <[email protected]>
PDCurses (Windows) assigns different numbers to color constants than ncurses (e.g. COLOR_RED=4 vs 1). Use Color["red"] etc. to resolve platform-appropriate values in assertions. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Themes can now declare default foreground/background colors via t.default_colors(foreground:, background:) which calls Curses.assume_default_colors during activation. Themes without default_colors reset to terminal defaults (-1, -1).